Skip to content

Conversation

@xdustinface
Copy link
Collaborator

@xdustinface xdustinface commented Feb 8, 2026

Use a tokio::time::Interval in the maintenance loop's select! instead of manual SystemTime elapsed tracking.
The interval arm is disabled in exclusive mode via a select guard and skips DNS if peers are already enough.

Also, the DNS lookup branch doesn't try to connect immediately but instead in the next loop cycle, which happens straight away.

Based on:

Summary by CodeRabbit

  • Refactor
    • Simplified peer discovery mechanism: now uses periodic DNS refresh intervals instead of time-based tracking for network peer management.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 8, 2026

📝 Walkthrough

Walkthrough

The PeerNetworkManager struct's DNS peer discovery mechanism was refactored to replace time-tracked, on-demand searches with a periodic timer-driven approach. A peer_search_started field was removed, and DNS discovery now runs at regular intervals via a dns_interval timer within the maintenance loop.

Changes

Cohort / File(s) Summary
DNS Discovery Refactoring
dash-spv/src/network/manager.rs
Removed peer_search_started: Arc<Mutex<Option<SystemTime>>> field and replaced time-based DNS delay tracking with periodic dns_interval timer mechanism. Updated struct initialization, Clone implementation, and maintenance loop logic to use interval-driven DNS discovery instead of start-time tracking.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰✨ The DNS search hops along with grace,
No need to track each starting place!
A timer ticks at intervals so true,
Discovery periodic—fresh and new!
One field removed, the maintenance glows bright,
Peer connections dancing through the night! 🌙

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: simplifying DNS discovery fallback logic by replacing manual SystemTime tracking with a periodic interval mechanism.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/peer-search-started

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
dash-spv/src/network/manager.rs (1)

763-764: Use interval_at to defer the first DNS discovery probe by one interval period.

time::interval(DNS_DISCOVERY_DELAY) completes its first .tick() immediately, which would trigger DNS discovery on the very first maintenance loop iteration—potentially redundant with the immediate DNS discovery already performed in start() (lines 173–196). Defer the first tick by one period using interval_at:

Suggested change
-            let mut dns_interval = time::interval(DNS_DISCOVERY_DELAY);
+            let mut dns_interval =
+                time::interval_at(time::Instant::now() + DNS_DISCOVERY_DELAY, DNS_DISCOVERY_DELAY);

The is_connecting guard prevents duplicate connection attempts, so this is an efficiency improvement rather than a correctness issue.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xdustinface xdustinface force-pushed the refactor/peer-search-started branch from ada9bcc to 5601598 Compare February 8, 2026 15:02
Base automatically changed from fix/use-stored-peers to v0.42-dev February 9, 2026 22:56
Use a `tokio::time::Interval` in the maintenance loop's `select!`
instead of manual `SystemTime` elapsed tracking.
The interval arm is disabled in exclusive mode via a select guard and
skips DNS if peers are already enough.

Also, the DNS lookup branch doesn't try to connect immediately but instead in the next loop cycle, which happens straight away.
@xdustinface xdustinface force-pushed the refactor/peer-search-started branch from 5601598 to a8a124f Compare February 9, 2026 23:13
@xdustinface xdustinface marked this pull request as ready for review February 10, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant